-
Notifications
You must be signed in to change notification settings - Fork 688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable pretty-error page when using --experimental-local
#2249
Conversation
🦋 Changeset detectedLatest commit: 8d693a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/3514297871/npm-package-wrangler-2249 You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/2249/npm-package-wrangler-2249 Or you can use npx https://prerelease-registry.developers.workers.dev/runs/3514297871/npm-package-wrangler-2249 dev path/to/script.js Additional artifacts:npm install https://prerelease-registry.developers.workers.dev/runs/3514297871/npm-package-cloudflare-pages-shared-2249 |
eaa0ed9
to
2b0a588
Compare
Codecov Report
@@ Coverage Diff @@
## main #2249 +/- ##
==========================================
+ Coverage 71.76% 71.77% +0.01%
==========================================
Files 145 145
Lines 9474 9482 +8
Branches 2470 2464 -6
==========================================
+ Hits 6799 6806 +7
- Misses 2675 2676 +1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple comments, but overall looks good!
2b0a588
to
5cd1f52
Compare
Previously, when middleware was enabled, service worker user code was being bundled as part of the middleware facade application, not in the final bundling step with `watch` enabled. This meant changes to user code were not picked up. This change restructures the service worker middleware loader and bundling code as an IIFE, that gets injected into the final bundle. This also has the positive side effect that middleware internals aren't exposed to users.
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place.
5cd1f52
to
8d693a0
Compare
LGTM 🎉 |
When applying facades, we write intermediate files to a temporary directory and then build them with `esbuild`. This outputs intermediate source maps, which get merged in the final bundle step to produce a final source map. On macOS, `os.tmpdir()` looks like `/var/folders/.../.../T`. `/var` is a symlink to `/private/var`. Unfortunately, source maps end up assuming files in `/var`, not `/private/var`, leading to invalid stack traces like `/private/Users/.../d1-beta-facade.js:101:12`. This change uses `realpathSync` to fully-resolve all symlinks before we pass paths containing the temporary directory to `esbuild` for source map generation. We actually implemented this same change in #2249 for the middleware loader facade, but not the rest.
When applying facades, we write intermediate files to a temporary directory and then build them with `esbuild`. This outputs intermediate source maps, which get merged in the final bundle step to produce a final source map. On macOS, `os.tmpdir()` looks like `/var/folders/.../.../T`. `/var` is a symlink to `/private/var`. Unfortunately, source maps end up assuming files in `/var`, not `/private/var`, leading to invalid stack traces like `/private/Users/.../d1-beta-facade.js:101:12`. This change uses `realpathSync` to fully-resolve all symlinks before we pass paths containing the temporary directory to `esbuild` for source map generation. We actually implemented this same change in #2249 for the middleware loader facade, but not the rest.
This PR enables Miniflare 3's pretty-error page with middleware. See cloudflare/miniflare#436 for an explanation of why we need to do this in the first place. It also fixes an issue where enabling the middleware loader in service-worker Workers disabled file watching.